Skip to content

Added java implementation of Huffman compression #181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 30, 2018
Merged

Added java implementation of Huffman compression #181

merged 7 commits into from
Jun 30, 2018

Conversation

sklan
Copy link
Contributor

@sklan sklan commented Jun 30, 2018

No description provided.

@june128 june128 added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label Jun 30, 2018
}

class HuffmanTree {
private Map<String, Integer> map = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could use a more descriptive name

private Map<String, Integer> map = new HashMap<>();
private Map<String, String> codeBook = new HashMap<>(), reverseCodeBook = new HashMap<>();
private TreeNode root;
private String s;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, single-letter class members are usually not a good idea

for (int i = 0; i < s.length(); i++) {
String key = Character.toString(s.charAt(i));

if (!map.containsKey(key)) map.put(key, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the else branch has braces, so should the if



while (priorityQueue.size() > 1) {
TreeNode temp1 = priorityQueue.remove();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left and right are probably better names here

while (priorityQueue.size() > 1) {
TreeNode temp1 = priorityQueue.remove();
TreeNode temp2 = priorityQueue.remove();
TreeNode node;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least merge this declaration with the line below but you could also just dopriorityQueue.add(new TreeNode(...));

private TreeNode root;
private String s;

public HuffmanTree(String s) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probay use better variable names for the constructor too


}

private void traverse(TreeNode temp, String w) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once more, these argument names are not the best.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, a StringBuilder would be much better here

private void traverse(TreeNode temp, String w) {
if (temp.left == null && temp.right == null)
codeBook.put(temp.key, w);
if (temp.left != null) traverse(temp.left, w + 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you can add integers to strings. Make sure to always test if your program compiles and works before submitting a PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The program is compiling fine. You can add integers to string in Java. I'll use StringBuilder as you say.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, you're right about the addition. Weird, not many languages allow that and Java's usually pretty strict (I've never seen it used in practice either)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was shocked too when I first learned about it. But you are right it creates unnecessary confusion.


public String encode() {
traverse(root, "");
String enc = "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A StringBuilder would be more efficoentt here

}

public String decode(String enc) {
String dec = "", key = "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, a StringBuilder and variable name suggestion. Since this is an educational resource I would go with encoded/decoded instead of enc/dec

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll change the variable names and make them more descriptive.

Copy link
Contributor

@zsparal zsparal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add the implementation to the chapter as well, otherwise it won't show up :)

@sklan
Copy link
Contributor Author

sklan commented Jun 30, 2018

Ok, done.

@zsparal zsparal merged commit 69ccdd6 into algorithm-archivists:master Jun 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants